Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client: inform all subscription handler about connection lose #1057

Conversation

schroeder-
Copy link
Contributor

If the watchdog task gets an error all subscription handler with status_change_notification method will be called with BadShutdown.
Also only prints an info message if the connection is lost.

@oroulet
Copy link
Member

oroulet commented Sep 29, 2022

I am wondering if we could add the same thing to clients subscribed to data changes. Ideally would the handler be called immediately with something raising an exception. But we should avoid breaking API if we can... or we break it and release 1.0. now that we have some safety in system it might be worth releasing 1.0 and it will make following correct versioning scheme easier.
We could add an error argument to datachange_notification which is None by default for example, although I am not that keen in that souiton

@schroeder-
Copy link
Contributor Author

I don't know because we already split between datachange and event. So I thought using status_change_notification can be used.
Or as an alternative we could use a new method to the handler:

def on_connection_state(self, state):
     .....

Where state can be a statuscode or our own enum. Like Connected, Disconnected, SubscriptionCreated, SubscriptionDeleted.
This could be used later when we implement a automatic reconnect to inform the clients that we are reconnecting.

@oroulet
Copy link
Member

oroulet commented Sep 30, 2022

or simply add on_connect() and on_disconnect() or on_connection_lost() to the Handler?
Not sure what is best
if we have only one method then on_connection_event(event)is probably a better name

@oroulet oroulet merged commit 602249d into FreeOpcUa:master Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants